-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Move dataclass kw_only fields to the end of the signature #19018
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: master
Are you sure you want to change the base?
Move dataclass kw_only fields to the end of the signature #19018
Conversation
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the found_dataclass_supertype
logic confusing -- why was that there?
Because this makes sense.
Given that we had a test asserting such behavior, I suspect #13539 was just a rushed implementation with some cases mistreated or misunderstood. I can't find any explanation to sorting the attributes only if there was a parent class - perhaps the author assumed that |
Will this PR also fix #17564? |
No, it won't, because # Follows the "jumping" logic of kw_only
def __init__(self, y: str, *, x: int): ...
# Definition order, reverse MRO order, no keywords
def __post_init__(self, x: int, y: str, /): ... (a bit more difficult with defaults, but similar still)
|
Thanks for explaining! |
#17564 is an easy fix, let's merge this and I will open a follow-up PR. |
Fixes #19017. Fixes #17731.
This is a rather naive change: python does that at runtime.
kw_only
args can be in any order, and non-kwonly args should remain sorted as-is (stable sort). I don't understand why this was only done in presence of a parent dataclass - AFAIC kwonly fields work that way sincekw_only
was introduced in py3.10.The test I changed was invalid and asserted a false positive to the best of my knowledge.